Skip to content

fix: discard without snapshots, move SLI to settings, fix version table#67

Merged
TerrifiedBug merged 3 commits intomainfrom
fix/ui-bugs-batch
Mar 8, 2026
Merged

fix: discard without snapshots, move SLI to settings, fix version table#67
TerrifiedBug merged 3 commits intomainfrom
fix/ui-bugs-batch

Conversation

@TerrifiedBug
Copy link
Copy Markdown
Owner

Summary

  • Discard changes: Now works for pipeline versions created before snapshot support was added — restores globalConfig, skips node/edge restore gracefully instead of throwing an error
  • SLI badge: Removed from toolbar (was cluttering it). Per-SLI health status indicators (green/red/gray dot) now show in Settings > Health SLIs
  • Version history: Fixed column header overlap ("Changelog" and "Created" were touching) caused by max-w-0 on the Changelog column

Test plan

  • Deploy a pipeline, make config changes, verify discard works
  • Open Settings gear > Health SLIs, verify per-SLI health dots show
  • Open version history dialog, verify column headers are properly spaced

…le columns

- Make discard changes work for versions without snapshots (restores
  globalConfig only, skips node/edge restore gracefully)
- Remove SLI health badge from toolbar to reduce clutter; add per-SLI
  health status indicators (green/red dot) in Settings > Health SLIs
- Fix version history table header overlap caused by max-w-0 on the
  Changelog column; move truncation to cell content instead
@github-actions github-actions bot added the fix label Mar 8, 2026
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 8, 2026

Greptile Summary

This PR addresses three independent bug fixes in the pipeline editor: graceful handling of pre-snapshot discardChanges, relocation of the SLI health indicator from the toolbar into Settings, and a CSS fix for the version history table column overlap.

  • Discard without snapshots (pipeline.ts): The old code hard-blocked discards if nodesSnapshot/edgesSnapshot were absent. The new code degrades gracefully — globalConfig is always restored; node/edge restoration is skipped when no snapshot is present. The transaction structure and existing withTeamAccess + withAudit middleware are preserved correctly.
  • SLI health indicator move (pipeline-settings.tsx / flow-toolbar.tsx): The aggregate SLI badge is removed from the toolbar; per-SLI green/red/gray status dots are now shown inline in Settings > Health SLIs. A behavioral regression was introduced: the health query enabled condition is weaker than the original (slis.length > 0 vs. slis.length > 0 && !isDraft && !!deployedAt), which causes unnecessary failed requests on undeployed pipelines that have SLIs defined.
  • Version history column fix (version-history-dialog.tsx): max-w-0 removed from the Changelog column header and cell; Version column widened from 80px to 120px. The truncation is now applied via max-w-[200px] on the inner span, which is the correct approach.

Confidence Score: 4/5

  • Safe to merge — all three fixes are correct and well-scoped with one behavioral regression in the health query guard that should be addressed before merge.
  • The backend discard logic is sound, authorization middleware is intact, and the UI fixes are straightforward. The only finding is that the relocated health query in SliSettings omits the isDraft/deployedAt guard that the original toolbar version had, which causes unnecessary failed API calls for undeployed pipelines but does not cause crashes or data loss.
  • src/components/flow/pipeline-settings.tsx — health query enabled condition is weaker than the original.

Last reviewed commit: e1fd3c2

Comment on lines +334 to +338
const healthQuery = useQuery(
trpc.pipeline.health.queryOptions(
{ pipelineId },
{ enabled: slis.length > 0, refetchInterval: 30_000 },
),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Health query fires on draft/undeployed pipelines

The original toolbar version of this query guarded with !isDraft && !!deployedAt before enabling the health endpoint. The new SliSettings component only checks slis.length > 0, so if a pipeline has SLIs configured but has never been deployed (or is still a draft), this query will fire and likely return an error or empty payload from the server.

The UI handles the missing data gracefully (healthQuery.data?.slis ?? []), so there's no crash — but it will generate unnecessary failed requests every 30 seconds as long as the Settings panel is open.

Consider passing isDraft and deployedAt into SliSettings and mirroring the original guard:

Suggested change
const healthQuery = useQuery(
trpc.pipeline.health.queryOptions(
{ pipelineId },
{ enabled: slis.length > 0, refetchInterval: 30_000 },
),
const healthQuery = useQuery(
trpc.pipeline.health.queryOptions(
{ pipelineId },
{ enabled: slis.length > 0 && !isDraft && !!deployedAt, refetchInterval: 30_000 },
),
);
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/components/flow/pipeline-settings.tsx
Line: 334-338

Comment:
Health query fires on draft/undeployed pipelines

The original toolbar version of this query guarded with `!isDraft && !!deployedAt` before enabling the health endpoint. The new `SliSettings` component only checks `slis.length > 0`, so if a pipeline has SLIs configured but has never been deployed (or is still a draft), this query will fire and likely return an error or empty payload from the server.

The UI handles the missing data gracefully (`healthQuery.data?.slis ?? []`), so there's no crash — but it will generate unnecessary failed requests every 30 seconds as long as the Settings panel is open.

Consider passing `isDraft` and `deployedAt` into `SliSettings` and mirroring the original guard:

```suggestion
  const healthQuery = useQuery(
    trpc.pipeline.health.queryOptions(
      { pipelineId },
      { enabled: slis.length > 0 && !isDraft && !!deployedAt, refetchInterval: 30_000 },
    ),
  );
```

How can I resolve this? If you propose a fix, please make it concise.

Return partial flag from discardChanges so the frontend can distinguish
full restore (with snapshots) from partial (globalConfig only). Shows a
toast explaining node configs weren't restored and to redeploy for full
discard support.
@TerrifiedBug TerrifiedBug merged commit 785d74a into main Mar 8, 2026
9 checks passed
@TerrifiedBug TerrifiedBug deleted the fix/ui-bugs-batch branch March 8, 2026 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant